Skip to content

Fixes #5239. AOT configuration initialization crash#5240

Merged
tig merged 7 commits intodevelopfrom
copilot/fix-aot-init-crash
May 7, 2026
Merged

Fixes #5239. AOT configuration initialization crash#5240
tig merged 7 commits intodevelopfrom
copilot/fix-aot-init-crash

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 7, 2026 •
edited
Loading

Fixes

AOT-published apps could crash before Main because configuration initialization deep-cloned Dictionary<Command, PlatformKeyBinding> via reflective construction after trimming removed the closed generic default constructor.

Proposed Changes/Todos

  • Clone hard-coded key binding dictionaries through a typed path instead of DeepCloner reflection.
  • Deep-clone PlatformKeyBinding entries and nested Key [] arrays while preserving the dictionary comparer.
  • Use a record with expression when cloning PlatformKeyBinding so future record properties are preserved.
  • Add regression coverage proving cached hard-coded key binding dictionaries are independent deep copies.
  • Stabilize the regression test by asserting/restoring configuration and application static state.
  • Update the key-array test helper to accept nullable arrays matching PlatformKeyBinding properties.

Pull Request checklist:

  • I've named my PR in the form of "Fixes #issue. Terse description."
  • My code follows the style guidelines of Terminal.Gui - if you use Visual Studio, hit CTRL-K-D to automatically reformat your files before committing.
  • My code follows the Terminal.Gui library design guidelines
  • I ran dotnet test before commit
  • I have made corresponding changes to the API documentation (using /// style comments)
  • My changes generate no new warnings
  • I have checked my code and corrected any poor grammar or misspellings
  • I conducted basic QA to assure all features are working

Copilot AI assigned Copilot and tig May 7, 2026
Copilot AI linked an issue May 7, 2026 that may be closed by this pull request
AOT: ConfigurationManager.Initialize crashes with MissingMethodException on Dictionary<Command, PlatformKeyBinding> #5239
Closed
@tig
Fix AOT hard-coded key binding clone
bc7cf72 
Agent-Logs-Url: https://github.com/gui-cs/Terminal.Gui/sessions/275ee9b0-ace3-402a-a75e-18ca75db4388

Co-authored-by: tig <585482+tig@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix AOT ConfigurationManager.Initialize MissingMethodException Fix AOT configuration initialization crash May 7, 2026
Copilot AI requested a review from tig May 7, 2026 18:29
Copilot finished work on behalf of tig May 7, 2026 18:29
@tig tig changed the title Fix AOT configuration initialization crash Fixes #5239 - AOT configuration initialization crash May 7, 2026
@tig tig requested a review from Copilot May 7, 2026 18:40
Copilot AI reviewed May 7, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes an AOT startup crash in ConfigurationManager.Initialize by avoiding reflection-based cloning of Dictionary<Command, PlatformKeyBinding> (which can fail after trimming removes the closed generic ctor). The PR introduces a typed deep-clone path for key binding dictionaries and adds a regression test asserting cached defaults are independent copies.

Changes:

  • Replaced DeepCloner.DeepClone(...) with a typed cloning path for Dictionary<Command, PlatformKeyBinding> during hard-coded configuration cache initialization.
  • Implemented deep-copy logic for PlatformKeyBinding and nested Key[] arrays while preserving the dictionary comparer.
  • Added a non-parallelizable test covering deep-copy independence of the cached key binding dictionaries.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
Terminal.Gui/Configuration/ConfigurationManager.cs Adds typed cloning for key binding dictionaries to avoid reflection/trim issues under AOT.
Tests/UnitTests.NonParallelizable/Configuration/ConfigurationMangerTests.cs Adds regression test ensuring cached hard-coded key binding dictionaries are deep copies.

Comment thread Tests/UnitTests.NonParallelizable/Configuration/ConfigurationMangerTests.cs Outdated
Comment thread Tests/UnitTests.NonParallelizable/Configuration/ConfigurationMangerTests.cs Outdated
Comment thread Terminal.Gui/Configuration/ConfigurationManager.cs
@tig
Potential fix for pull request finding
6041868 
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI and others added 2 commits May 7, 2026 19:00
Copilot AI changed the title Fixes #5239 - AOT configuration initialization crash Fixes #5239. AOT configuration initialization crash May 7, 2026
Copilot finished work on behalf of tig May 7, 2026 19:03
Copilot finished work on behalf of tig May 7, 2026 19:06
@tig tig marked this pull request as ready for review May 7, 2026 19:17
@tig tig requested review from BDisp and harder May 7, 2026 19:17
@tig
Add AOT-safety comment to CloneHardCodedPropertyValue
bfc9c73 
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@tig
Copy link
Copy Markdown
Member

tig commented May 7, 2026

I had 4 different agents review this and I ran tests exhausively. I'm going to break the rule of having another maintainer review it becauswe I need it integrated now so I can make progress on validating AOT with clet.

@tig tig merged commit 86c3fde into develop May 7, 2026
11 checks passed
@tig tig deleted the copilot/fix-aot-init-crash branch May 7, 2026 19:38
@tig tig mentioned this pull request May 7, 2026
Closed
Copilot AI mentioned this pull request May 7, 2026
Merged
11 tasks
tig added a commit that referenced this pull request May 7, 2026
@tig
Fixes #5239. AOT configuration initialization crash (#5240)
bf0621c 
* Initial plan

* Fix AOT hard-coded key binding clone

Agent-Logs-Url: https://github.com/gui-cs/Terminal.Gui/sessions/275ee9b0-ace3-402a-a75e-18ca75db4388

Co-authored-by: tig <585482+tig@users.noreply.github.com>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Match nullable key array test helper

Agent-Logs-Url: https://github.com/gui-cs/Terminal.Gui/sessions/d1e76594-a3c8-4c84-b738-3decfdb73987

Co-authored-by: tig <585482+tig@users.noreply.github.com>

* Use precise nullable warning suppression

Agent-Logs-Url: https://github.com/gui-cs/Terminal.Gui/sessions/d1e76594-a3c8-4c84-b738-3decfdb73987

Co-authored-by: tig <585482+tig@users.noreply.github.com>

* Use record with expression for key binding clone

Agent-Logs-Url: https://github.com/gui-cs/Terminal.Gui/sessions/094cf268-64cd-42e9-9179-af0d70e786a2

Co-authored-by: tig <585482+tig@users.noreply.github.com>

* Add AOT-safety comment to CloneHardCodedPropertyValue

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: tig <585482+tig@users.noreply.github.com>
Co-authored-by: Tig <tig@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@BDisp
Copy link
Copy Markdown
Collaborator

BDisp commented May 7, 2026

I had 4 different agents review this and I ran tests exhausively. I'm going to break the rule of having another maintainer review it becauswe I need it integrated now so I can make progress on validating AOT with clet.

No problem. Feel free because you know very well what you're doing, and very well. Often we don't have access to a laptop either. But I saw the PR just now and they actually adopted a very clever solution by only forcing deep cloning on untreated cases and speed cloning on the remaining cases.

@tig tig mentioned this pull request May 7, 2026
Merged
tig added a commit that referenced this pull request May 7, 2026
@tig @claude
Restore #5243 AOT fix lost in #5246 merge resolution (#5249)
ea205f3 
The fix from #5243 ("Fixes #5259. AOT: use source-generated JSON
serializer for dictionary construction in DeepCloner") was reverted
when release/v2.1.0-rc.1 merged origin/main back at 94dca98. That
merge had conflicts in DeepCloner.cs and ConfigurationManager.cs and
the resolution picked main's pre-#5243 versions, dropping:

  - 27 lines from DeepCloner.cs (the SourceGenerationContext.GetTypeInfo
    fallback in CreateDictionaryInstance, plus its leading comment)

and re-adding 54 lines to ConfigurationManager.cs:

  - CloneHardCodedPropertyValue and the per-type clone helpers
    (CloneKeyBindings, ClonePlatformKeyBinding, CloneKeyArray) that
    #5243 had removed in favor of generic DeepCloner handling
  - the call site change from DeepClone to CloneHardCodedPropertyValue

PRs #5246 (rc.1 → main) and #5247 (rc.1 backmerge → develop) carried
the broken state forward. v2.1.0-beta.1 was tagged before the merge
(no fix); v2.1.0-rc.1 was tagged after but doesn't have it either.

Repro: `dotnet publish Examples/NativeAot -c Release -r win-x64
--self-contained -p:PublishAot=true` against current develop crashes
at module init with `MissingMethodException: No parameterless
constructor defined for type Dictionary<ColorName16, string>`. Same
crash a downstream AOT consumer (clet) hit and tracked as #5239 →
#5240 → #5242 → #5243 → #5248.

This commit checks DeepCloner.cs and ConfigurationManager.cs out of
11d6e27 (the original PR #5243 merge), restoring the source-gen
fallback and removing the per-type helpers. Verified locally:

  - `dotnet build Terminal.Gui/Terminal.Gui.csproj -c Release` → 0 errors
  - clet AOT publish + `clet.exe --version` runs cleanly (was crashing
    against rc.1; now exits 0 with the local TG nupkg pinned)

Refs #5239, #5242, #5243, #5248, #5259.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@dependabot dependabot Bot mentioned this pull request May 8, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@harder harder Awaiting requested review from harder

@BDisp BDisp Awaiting requested review from BDisp

+1 more reviewer

@tig tig tig approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@tig tig

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AOT: ConfigurationManager.Initialize crashes with MissingMethodException on Dictionary<Command, PlatformKeyBinding>

4 participants

@tig @BDisp